Skip to content

Conversation

a-tarasyuk
Copy link
Member

Fixes #62588

@a-tarasyuk a-tarasyuk requested a review from Endilll as a code owner November 10, 2024 13:12
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 10, 2024

@llvm/pr-subscribers-clang

Author: Oleksandr T. (a-tarasyuk)

Changes

Fixes #62588


Full diff: https://github.com/llvm/llvm-project/pull/115656.diff

5 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/include/clang/Sema/Sema.h (+1-1)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+10-5)
  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+1-1)
  • (modified) clang/test/SemaCXX/warn-shadow.cpp (+14)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c3424e0e6f34c9..f8005c57ea865c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -527,6 +527,8 @@ Improvements to Clang's diagnostics
 
 - Clang now diagnoses ``[[deprecated]]`` attribute usage on local variables (#GH90073).
 
+- Clang now omits shadow warnings for enum constants in separate class scopes (#GH62588).
+
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fad446a05e782f..8e8097e86764c6 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3453,7 +3453,7 @@ class Sema final : public SemaBase {
   ///
   /// \param ShadowedDecl the declaration that is shadowed by the given variable
   /// \param R the lookup of the name
-  void CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
+  void CheckShadow(Scope *S, NamedDecl *D, NamedDecl *ShadowedDecl,
                    const LookupResult &R);
 
   /// Check -Wshadow without the advantage of a previous lookup.
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 61c29e320d5c73..7bf7a19de1c6a6 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6732,7 +6732,7 @@ Sema::ActOnTypedefNameDecl(Scope *S, DeclContext *DC, TypedefNameDecl *NewTD,
   }
 
   if (ShadowedDecl && !Redeclaration)
-    CheckShadow(NewTD, ShadowedDecl, Previous);
+    CheckShadow(S, NewTD, ShadowedDecl, Previous);
 
   // If this is the C FILE type, notify the AST context.
   if (IdentifierInfo *II = NewTD->getIdentifier())
@@ -8092,7 +8092,7 @@ NamedDecl *Sema::ActOnVariableDeclarator(
 
   // Diagnose shadowed variables iff this isn't a redeclaration.
   if (!IsPlaceholderVariable && ShadowedDecl && !D.isRedeclaration())
-    CheckShadow(NewVD, ShadowedDecl, Previous);
+    CheckShadow(S, NewVD, ShadowedDecl, Previous);
 
   ProcessPragmaWeak(S, NewVD);
 
@@ -8237,7 +8237,7 @@ NamedDecl *Sema::getShadowedDeclaration(const BindingDecl *D,
                                                             : nullptr;
 }
 
-void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
+void Sema::CheckShadow(Scope *S, NamedDecl *D, NamedDecl *ShadowedDecl,
                        const LookupResult &R) {
   DeclContext *NewDC = D->getDeclContext();
 
@@ -8341,6 +8341,11 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
     // shadowing context, but that's just a false negative.
   }
 
+  // Skip shadowing check if we're in a class scope, dealing with an enum
+  // constant in a different context.
+  if (S->isClassScope() && isa<EnumConstantDecl>(D) &&
+      !OldDC->Equals(NewDC->getRedeclContext()))
+    return;
 
   DeclarationName Name = R.getLookupName();
 
@@ -8389,7 +8394,7 @@ void Sema::CheckShadow(Scope *S, VarDecl *D) {
                  RedeclarationKind::ForVisibleRedeclaration);
   LookupName(R, S);
   if (NamedDecl *ShadowedDecl = getShadowedDeclaration(D, R))
-    CheckShadow(D, ShadowedDecl, R);
+    CheckShadow(S, D, ShadowedDecl, R);
 }
 
 /// Check if 'E', which is an expression that is about to be modified, refers
@@ -19736,7 +19741,7 @@ Decl *Sema::ActOnEnumConstant(Scope *S, Decl *theEnumDecl, Decl *lastEnumConst,
   if (PrevDecl) {
     if (!TheEnumDecl->isScoped() && isa<ValueDecl>(PrevDecl)) {
       // Check for other kinds of shadowing not already handled.
-      CheckShadow(New, PrevDecl, R);
+      CheckShadow(S, New, PrevDecl, R);
     }
 
     // When in C++, we may get a TagDecl with the same name; in this case the
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 8d76a35b2d2557..ad8df19485d9da 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -924,7 +924,7 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator &D,
         Diag(Old->getLocation(), diag::note_previous_definition);
       }
     } else if (ShadowedDecl && !D.isRedeclaration()) {
-      CheckShadow(BD, ShadowedDecl, Previous);
+      CheckShadow(S, BD, ShadowedDecl, Previous);
     }
     PushOnScopeChains(BD, S, true);
     Bindings.push_back(BD);
diff --git a/clang/test/SemaCXX/warn-shadow.cpp b/clang/test/SemaCXX/warn-shadow.cpp
index 2969bd39fed41e..98a235a73c7e52 100644
--- a/clang/test/SemaCXX/warn-shadow.cpp
+++ b/clang/test/SemaCXX/warn-shadow.cpp
@@ -307,3 +307,17 @@ void test4() {
 }
 
 }; // namespace structured_binding_tests
+
+namespace GH62588 {
+class Outer {
+public:
+  char *foo();          // expected-note {{previous declaration is here}} \
+                        // expected-note {{previous definition is here}}
+  enum Outer_E { foo }; // expected-error {{redefinition of 'foo'}} \
+                        // expected-warning {{declaration shadows a static data member of 'GH62588::Outer'}}
+  class Inner {
+  public:
+    enum Inner_E { foo }; // ok
+  };
+};
+} // namespace GH62588

@a-tarasyuk a-tarasyuk requested a review from Fznamznon November 12, 2024 15:12
Copy link
Contributor

@Fznamznon Fznamznon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, in case @Sirraide and @AaronBallman don't have any further comments.

@Sirraide Sirraide merged commit 738a047 into llvm:main Nov 19, 2024
9 checks passed
@michael-jabbour-sonarsource
Copy link
Contributor

michael-jabbour-sonarsource commented Jul 2, 2025

Hello,

I think that the following cases used to be valid reports on clang 19, but they are not reported in clang 20 after this patch. Is this intended?

namespace classEnums {
int globalVar = 0;
class ClassWithEnum {
  enum Enum {
    globalVar  // No -Wshadow after this patch.
  };
};

class OuterClass {
  static const int memberData = 0;
  enum Enum {
    enumVal
  };
  class InnerClass {
    enum Enum {
      memberData,  // No -Wshadow after this patch.
      enumVal  // No -Wshadow after this patch.
    };
  };
};
}

CE: https://godbolt.org/z/no87EEYxn

@a-tarasyuk, @Fznamznon, @Sirraide

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong diagnostic about shadowing in nested class
5 participants